Parametrize acceptance action with pytest parallel worker count#730
Merged
Parametrize acceptance action with pytest parallel worker count#730
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds configurability for pytest-xdist parallelism in the acceptance GitHub Action and reduces staticcheck noise from a tracked deprecation migration.
Changes:
- Introduce a new
naction input (default10) and propagate it viaPYTEST_Ninto the acceptance test runtime. - Update
pytest_run.pyto read the worker count fromPYTEST_Ninstead of hard-coding-n 10. - Add an
acceptance/staticcheck.confto suppressSA1019warnings duringmake fmtuntil a separate migration completes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| acceptance/action.yml | Adds the new n input with default 10. |
| acceptance/main.go | Forwards the n input into the action’s environment as PYTEST_N. |
| acceptance/ecosystem/pytest_run.py | Uses PYTEST_N to set pytest-xdist -n. |
| acceptance/staticcheck.conf | Globally disables SA1019 for the acceptance module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses PR #730 review: - Trim + validate `n` in main.go as positive integer or "auto", returning a clear error for invalid values instead of letting pytest-xdist fail with an opaque CLI error. - Treat an empty/whitespace PYTEST_N env var as unset in pytest_run.py so `-n ''` is never passed to pytest. Co-authored-by: Isaac
pytest_run.py already strips and defaults PYTEST_N, and pytest-xdist surfaces clear CLI errors for non-integer values. Remove the Go-side validation so the action is a simple pass-through — this also keeps `n: 'auto'` working as a pytest-xdist native mode. Co-authored-by: Isaac
Exercises the n → PYTEST_N wiring when the skipped integration test is run manually with real credentials. Co-authored-by: Isaac
alexott
approved these changes
Apr 28, 2026
Contributor
Author
|
@alexott Let me also test manually e2e. I will ping you when it's done |
Contributor
Author
|
@alexott this is ready to merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ninput to the acceptance GitHub Action (default10) for configuring pytest-xdist parallel workers. Propagates via thePYTEST_Nenv var topytest_run.py; Go tests are unaffected.make fmtTest plan
make fmtpasses locallymake testpasses locallyn: '4'and confirm pytest uses-n 4nand confirm default of10Context: we need to be able to reduce parallel test execution in DQX as 10 parallel runs creates a high contention of testing clusters. This would require a new release.